Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Tests unique_within_region with class that uses STI #16740

Closed
wants to merge 1 commit into from

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Jan 3, 2018

This addresses a concern that the CustomizationTemplate class uses STI and thus the unique_within_region validation (that currently uses record.class) being applied to it may not properly scope across leaf classes. The suggestion was to use record.class.base_class instead.

Depends on

#16775

@miq-bot miq-bot added the wip label Jan 3, 2018
@d-m-u d-m-u changed the title [WIP] Allows us to properly scope for unique_within_region [WIP] Tests customization template unique_within_region scope Jan 4, 2018
@d-m-u d-m-u changed the title [WIP] Tests customization template unique_within_region scope Tests customization template unique_within_region scope Jan 4, 2018
@miq-bot miq-bot removed the wip label Jan 4, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 4, 2018

@miq-bot add_label core

context "two pxe templates" do
it "raises error with non-unique names" do
expect { @template = FactoryGirl.create(:customization_template, :name => "template") }.to_not raise_error
expect { @template1 = FactoryGirl.create(:customization_template, :name => @template.name) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@template1 is not used. You can remove.

@d-m-u d-m-u force-pushed the fix_for_issue_16739 branch 6 times, most recently from b8e33d0 to 2ad96cd Compare January 8, 2018 21:54
@d-m-u d-m-u closed this Jan 9, 2018
@d-m-u d-m-u reopened this Jan 9, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 9, 2018

The CustomizationTemplate isn't a model that you can create just the base class of. So this model isn't applicable to the issue that it was originally created for. Instead, #16775 fixes #16739 and this did still need tests. The removal of the scope may still be causing issues with the tests.

@d-m-u d-m-u force-pushed the fix_for_issue_16739 branch 3 times, most recently from b04a4e9 to ec2aaf4 Compare January 9, 2018 20:46
@Fryguy
Copy link
Member

Fryguy commented Jan 9, 2018

The CustomizationTemplate isn't a model that you can create just the base class of

No, but I would expect that creating two separate leaf classes with the same name should probably fail the uniqueness check as well.

@d-m-u d-m-u changed the title Tests customization template unique_within_region scope [WIP] Tests customization template unique_within_region scope Jan 10, 2018
@miq-bot miq-bot added the wip label Jan 10, 2018
@d-m-u d-m-u force-pushed the fix_for_issue_16739 branch 3 times, most recently from 5c2adef to 093b3f5 Compare January 10, 2018 17:21
@d-m-u d-m-u changed the title [WIP] Tests customization template unique_within_region scope [WIP] Tests unique_within_region with class that uses STI Jan 10, 2018
@miq-bot
Copy link
Member

miq-bot commented Jan 10, 2018

Checked commit d-m-u@b8402fa with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🏆

@d-m-u d-m-u deleted the fix_for_issue_16739 branch October 25, 2018 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants